-
Notifications
You must be signed in to change notification settings - Fork 6
Contract testing using testrunner and evmone #276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Allow overriding paths and commands via environment variables: - SOLCORE_CMD (default: cabal exec sol-core --) - YULE_CMD (default: cabal run yule --) - testrunner_exe (default: test/testrunner/testrunner) - evmone (default: ~/.local/lib/libevmone.so) This preserves the ability to run scripts outside of Nix builds while allowing Nix to override these values as needed.
Documents: - C++ testrunner architecture and components - Contest integration test system - Environment variable configuration - Nix build system and packages - Development workflow for local and Nix builds
Documents: - Building the C++ testrunner with cmake - Running integration tests manually and via Nix - Distinction between Haskell unit tests and integration tests - Test case structure and usage - Using contest.sh for individual test execution
Adds parallel 'integration-tests' job that runs 'nix flake check' to execute the C++ testrunner and contest integration tests on every PR. The workflow now has two parallel jobs: - build: Builds sol-core (existing) - integration-tests: Runs full pipeline tests (new)
Removes branch restriction from pull_request trigger so the workflow runs on all PRs regardless of target branch. Keeps push trigger restricted to main branch to avoid duplicate runs.
Replaces separate 'build' and 'integration-tests' jobs with a single 'build-and-test' job that runs 'nix flake check'. This is more efficient because: - nix flake check builds sol-core as a dependency (no duplicate builds) - Single job is simpler and clearer - Still runs full integration test suite
Y-Nak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added two nits.
Also, currently, the tests fail because of the copyToMem implementation. But other than that, LGTM overall.
| fi | ||
|
|
||
| echo "Processing: $file" | ||
| root_dir="$(cd "$(dirname "$(readlink --canonicalize "${BASH_SOURCE[0]}")")" && pwd)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| root_dir="$(cd "$(dirname "$(readlink --canonicalize "${BASH_SOURCE[0]}")")" && pwd)" | |
| root_dir="$(cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")" && pwd)" |
readlink -f doesn't work in macos(if coreutils are not installed). Probably realpath would be more portable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readlink -fdoesn't work in macos(ifcoreutilsare not installed). Probablyrealpathwould be more portable.
Does realpath work on macos without coreutils? On my (admittedly ancient - macOS 10.13) MacBook it is the oposite - system readlink and realpath from coreutils.
Is readlink an issue for you even when using nix develop?
We can always bemore paranoid and try
root_dir="$(realpath "$(dirname "${BASH_SOURCE[0]}")" || cd "$(dirname "$(readlink --canonicalize "${BASH_SOURCE[0]}")")" && pwd)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
realpath works on macos without coreutils (probably since around 10.15?). But nix develop and nix flake check works well with readlink --canonical. So it'd be fine to leave it as is for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how this is relevant to the pr, perhaps split it into a separate one?
| **Packages** (`nix build .#<package>`): | ||
| - `sol-core` - Main Haskell compiler | ||
| - `testrunner` - C++ testrunner binary | ||
| - `intx`, `blst`, `evmone` - EVM dependencies (built from source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why though? intx and blst are pulled by evmone's cmake in a pinned version - the supported and tested combination, I would assume. Can't we just rely on that instead of potentially introducing version drift?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also i'm not sure what happens with nlohmann_json here, is Hunter completely disabled, which version does it take?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clonker I am no expert on Nix, but from what I am told, in Nix builds, you're in a sandbox with no internet access. So Hunter can't download packages. That's why the current evmone.nix:
- Disables Hunter completely (lines 24-32) by stubbing out the Hunter macros
- Provides intx and blst explicitly from Nix instead
Can you suggest a better approach that works with Nix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't, I'm not a Nix user at all; it just struck me as odd and potentially unstable / hard to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I think it's fine. We just shouldn't forget to update intx and blst together with evmone. I'm not sure about the specifics but perhaps we can encapsulate the intx, blst, and nlohmann_json deps in the evmone package so they're not top-level? They're not runtime dependencies but statically included in the binary.
Which tests fail exactly? It seems that nix flake check succeeds on github - see https://github.com/argotorg/solcore/actions/runs/20598930090/job/59159651508?pr=276 |
Co-authored-by: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Y-Nak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested one change to be able to run nix flake check on macos.
Co-authored-by: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
11d39b7 to
d89bc7e
Compare
be6f4ef to
df737d2
Compare
Contract testing (create contract and call its methods) using testrunner and evmone.
This PR integrates the C++ testrunner into the Nix build system and adds end-to-end integration tests to
nix flake check(which is now run as part of the CI workflow).Written using substantial Claude Code input, hence includes CLAUDE.md (which can also serve as a part of documentation).
Changes
Nix Build System:
Script Portability:
Testing:
Documentation:
Usage